Improve how identifiers are treated #84
Conversation
stIncMale
force-pushed
the
improveHowItentifiersAreTreated
branch
from
2ef6d47 to
4f9ef0e
Compare
|
|
||
| @Override | ||
| public @Nullable String toQuotedIdentifier(@Nullable String name) { | ||
| return name; |
There was a problem hiding this comment.
This single change makes all the new tests pass, and without it, they all fail. Nothing else related to quoting both in Dialect (including Dialect.buildIdentifierHelper) and in DatabaseMetaData had any observable effect on the tests added.
There was a problem hiding this comment.
It is a great news! Thanks for your PR. With some minor changes it could be merged.
| static final String COLLECTION_NAME = "\"collection in double quotes Mixed Case\""; | ||
| static final String FIELD_NAME = "\"field in double quotes Mixed Case\""; | ||
| static final String ACTUAL_COLLECTION_NAME = "collection in double quotes Mixed Case"; | ||
| static final String ACTUAL_FIELD_NAME = "field in double quotes Mixed Case"; |
There was a problem hiding this comment.
Unfortunately, it seems impossible to prevent Hibernate ORM from stripping identifiers like \"collection in double quotes Mixed Case\" of their quotes. Apparently, it has logic somewhere that ignores any configuration/overrides related to quoting, and strips double and single quotes anyway.
There was a problem hiding this comment.
That place seems to be here https://github.com/hibernate/hibernate-orm/blob/0a4d551b32801ae15e99d2f8ac0750303ecda2b6/hibernate-core/src/main/java/org/hibernate/boot/model/naming/Identifier.java#L159-L173 - it always considers identifiers enclosed in a set of statically-defined characters as quoted.
There was a problem hiding this comment.
yeah, it seems so. If the Hibernate quoting at the start and end is hard-coded or not configurable, I think we could remove those MixedCases testing cases for we don't need to test Hibernate if its behaviour is working as expected.
There was a problem hiding this comment.
The MixedCases tests are for testing that Hibernate ORM does not change the characters in collection/field names with mixed case to upper-/lower-case characters. Currently there is only one test (withSpaceAndDotAndMixedCase) left checking that. Do you still think we should not test this?
src/integrationTest/java/com/mongodb/hibernate/IdentifierIntegrationTests.java
Outdated
Show resolved
Hide resolved
|
|
||
| @Entity | ||
| @Table(name = InSingleQuotesMixedCase.COLLECTION_NAME) | ||
| static class InSingleQuotesMixedCase { |
There was a problem hiding this comment.
the MixedCase is a little bit unclear, though further code reading shows that the Mixed case simply means single quotes both at the beginning and at the end.
Optional. Avoid the confusing MixedCase here by using more intuitive verbiage like InSingleQuotesDelimiters or simply StartingAndEndingWithSingleQuote.
There was a problem hiding this comment.
though further code reading shows that the Mixed case simply means
single quotes both at the beginning and at the end.
No, "Mixed Case" means what is usually does - text containing both uppercase and lowercase letters. Since it wasn't clear, I removed it from all but one place in 96b8c78.
src/integrationTest/java/com/mongodb/hibernate/IdentifierIntegrationTests.java
Outdated
Show resolved
Hide resolved
src/integrationTest/java/com/mongodb/hibernate/IdentifierIntegrationTests.java
Outdated
Show resolved
Hide resolved
| static final String COLLECTION_NAME = "\"collection in double quotes Mixed Case\""; | ||
| static final String FIELD_NAME = "\"field in double quotes Mixed Case\""; | ||
| static final String ACTUAL_COLLECTION_NAME = "collection in double quotes Mixed Case"; | ||
| static final String ACTUAL_FIELD_NAME = "field in double quotes Mixed Case"; |
There was a problem hiding this comment.
yeah, it seems so. If the Hibernate quoting at the start and end is hard-coded or not configurable, I think we could remove those MixedCases testing cases for we don't need to test Hibernate if its behaviour is working as expected.
src/integrationTest/java/com/mongodb/hibernate/IdentifierIntegrationTests.java
Show resolved
Hide resolved
|
|
||
| @Override | ||
| public @Nullable String toQuotedIdentifier(@Nullable String name) { | ||
| return name; |
There was a problem hiding this comment.
It is a great news! Thanks for your PR. With some minor changes it could be merged.
src/integrationTest/java/com/mongodb/hibernate/IdentifierIntegrationTests.java
Show resolved
Hide resolved
…cks`/`StartingAndEndingWithDoubleQuotes`
No description provided.